Skip to content

fix: stability improvements batch 4 (Tier 2 cleanup)#2861

Merged
simonredfern merged 4 commits into
OpenBankProject:developfrom
hongwei1:fix/stability-batch4-upstream
Jul 13, 2026
Merged

fix: stability improvements batch 4 (Tier 2 cleanup)#2861
simonredfern merged 4 commits into
OpenBankProject:developfrom
hongwei1:fix/stability-batch4-upstream

Conversation

@hongwei1

Copy link
Copy Markdown
Contributor

Summary

Four independent Tier 2 stability fixes, one commit each:

  • fix(dates): shared SimpleDateFormat instances in APIUtil/Helper were not thread-safe (both parse and format mutate internal state); switched hot paths to ThreadLocal-backed accessors and low-frequency sites to per-call instances. Deleted unused DateFormatWithCurrentTimeZone.
  • fix(db): introduced BlockingIoExecutionContext, a dedicated bounded thread pool for blocking JDBC connection acquisition and gRPC dispatch, replacing ExecutionContext.global at 6 call sites so blocked I/O cannot starve request-handling Futures on the CPU-sized global pool.
  • fix(email): the password-reset endpoint discarded the Box[String] result of sending the reset email and always returned "sent", even when SMTP failed. Now surfaces SMTP failures as a 500 response; token generation still happens before the send attempt so a retry does not lose progress.
  • fix(ukob): 20 UK Open Banking v3.1 endpoint files build ResourceDoc example bodies via a private parseBody helper that returned a raw JObject; wrapped in JvalueCaseClass so swagger/resource-docs serialization takes the correct unwrap path (same bug class as the earlier Berlin Group v1.3 fix).

Test plan

  • mvn install -pl .,obp-commons -am -DskipTests — BUILD SUCCESS (JDK 25)
  • mvn compile -pl obp-api -am — BUILD SUCCESS
  • New unit test DateFormatConcurrencyTest (32 concurrent threads) — passed
  • PasswordResetTest full suite, including new SMTP-failure scenario — 17/17 passed
  • git grep -n 'new SimpleDateFormat' obp-api/src/main — no remaining shared singleton vals
  • git grep -n 'ExecutionContext.global' obp-api/src/main — the 6 target sites are gone
  • CI shard suites (UK OB v3.1, ResourceDocsTest/SwaggerDocsTest, code.api.util) — pending CI run on this PR

hongwei1 added 4 commits July 13, 2026 14:30
SimpleDateFormat's parse and format methods both mutate internal state;
shared singletons silently return wrong dates under concurrency. Replace
mutable shared instances with ThreadLocal-backed accessors on the hot
paths (APIUtil, Helper), and per-call defs for low-frequency sites
(ConsentScheduler, OBPDataImport).

Dead code removal: delete DateFormatWithCurrentTimeZone, which mutated
the shared format's timezone and is not called anywhere.

Add unit test hammering parseObpStandardDate and format/re-parse round-trip
from 32 concurrent threads to ensure all results are identical and correct.
ExecutionContext.global is CPU-sized, and blocking I/O (DataSource connection
acquisition via Doobie's connectEC, gRPC dispatch) on that pool starves all
request Futures waiting on the same small thread count.

Introduce BlockingIoExecutionContext, a fixed-size thread pool (default 50,
prop-overridable) dedicated to blocking JDBC and gRPC work. Replace global
at 6 sites:
- DoobieTransactor.scala (2 sites: fallback + update transactors)
- StoredProcedureUtils.scala (SP connector transactor)
- ProjectionDb.scala (projection transactor)
- ObpGrpcServer.scala (gRPC server dispatch)
- Boot.scala:1074 (production gRPC server creation)

Default pool size covers Hikari maximumPoolSize sum (main 20 + SP 20) with
headroom; override via blocking_io_pool.size prop if Hikari pools resize.
… sent

The password-reset endpoint (POST /management/user/reset-password-url) called
sendHtmlEmail and discarded the result, returning 'sent' unconditionally even
when the SMTP send failed. The caller (admin with canCreateResetPasswordUrl)
already knows the target user's email, so anti-enumeration does not apply here.

Switch to sendHtmlEmailEither and surface SMTP send failures as 500 responses
instead of lying that the email was sent. Restructure the for-comprehension to
hoist token generation before the send, and fail immediately on send failure.

Add test scenario: verify that when SMTP is misconfigured (closed port) and
test mode is off, the endpoint returns 500 with a message indicating the send
failed, not the successful 'sent' response.
Since the json4s migration, JValue itself extends scala.Product, so a raw
json.parse(...) result satisfies ResourceDoc example-body parameter types and
compiles. However, resource-docs serialization then reflects on it as a generic
case class instead of taking the JvalueCaseClass special-case unwrap path,
causing schema mismatches and potential asInstanceOf[JvalueCaseClass] failures.

All 20 affected UK OB v3.1 files share an identical private parseBody helper
used only for ResourceDoc example bodies (79 call sites total, never in handler
logic). Wrap the helper return in JvalueCaseClass(...) so serialization takes
the correct path.

This matches the earlier Berlin Group v1.3 fix (30afe2a) that resolved the
same root cause.
@sonarqubecloud

Copy link
Copy Markdown

@simonredfern simonredfern merged commit 5de5e01 into OpenBankProject:develop Jul 13, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants